Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

yasna & dependencies update #637

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mridul-manohar
Copy link

The currently used version of yasna is ver0.3, and it is advised to upgrade to latest yasna ver0.5.2.
This PR upgrades yasna version along with required dependencies.

Copy link
Collaborator

@Taowyoo Taowyoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there are some code also need to change to sync up with upstream changes.

url = "2.2"

mbedtls = { version = "0.12", features = ["std"], default-features = false, optional = true }
mbedtls = { version = "0.12", git = "https://github.com/fortanix/rust-mbedtls.git", branch = "mridul/yasna-0.5-crate-update", features = ["std"], default-features = false, optional = true }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you create an issue for tracking this, we should avoid using branch version?

@Taowyoo
Copy link
Collaborator

Taowyoo commented Aug 27, 2024

Regarding CI failure, I guess there should be a libc version change breaking vsock compilation.

@@ -10,24 +10,24 @@ categories = [ "api-bindings" ]
keywords = [ "sgx" ]

[dependencies]
b64-ct = "0.1.0"
b64-ct = "0.1.2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this version constraint? With the specification b64-ct = "0.1.0" users are free to update to 0.1.2 if they choose. The same comment applies to many/most other changes in this PR.
See https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-cratesio

em-client = { version = "3.0.0", default-features = false, features = ["client"] }
em-node-agent-client = "1.0.0"
hyper = { version = "0.10", default-features = false }
mbedtls = { version = "0.12", default-features = false, features = ["rdrand", "std", "ssl"] }
mbedtls = { version = "0.12", git = "https://github.com/fortanix/rust-mbedtls.git", branch = "mridul/yasna-0.5-crate-update", default-features = false, features = ["rdrand", "std", "ssl"] }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't specify dependencies by pointing to git repos. Does mbedtls even expose yasna types directly? If not, it could be updated to pick up yasna and be published as a patch update. Then users of em-app will pick yasna 0.5 as a dependency of mbedtls without any update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants